Post code suggestions on PRs with format issues #15477
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
@markpollack did I understand correctly you'd be looking to do the same on spring-ai through |
There was a problem hiding this comment.
I like it, @timtebeek. I've left some feedback inline. Also, since this will affect the whole team, I'll add team-attention and we can discuss at our next standup.
| # Post suggestions as a comment on the PR | ||
| - uses: googleapis/code-suggester@v4 | ||
| with: | ||
| command: review |
There was a problem hiding this comment.
Is there a way to include a comment in the patch suggestion? If so, it would be nice to inform the OP of ./gradlew format. I realize they can just accept the PR suggestion instead, I just see it as informative for the future so they remove one step from their submissions.
There was a problem hiding this comment.
We had wondered the same internall, but unfortunately it does not appear to be an option exposed when using the code-suggester review command: https://github.com/googleapis/code-suggester?tab=readme-ov-file#review-a-pull-request
There is an alternative in the form of reviewdog that's more malleable, but also adds branding and token requirements that I'm not too pleased with, which is why I stuck with the minimal API offered by code-suggester.
jzheaux
added
in: build
All good; perhaps worth bringing up that this approach can be extended to cover additional cases, like missing copyright headers, or even automated code changes, like we do on our projects. For simplicity I'd started small here though, to optionally extend later. |
|
@timtebeek, instead of code suggestions, could it try and push a polish commit to the PR's branch instead? If the push fails then post a comment to the PR indicating how to fix the formatting themselves. |
Hmm; haven't explored that option yet, and it'd take some care to be sure that this can be done safely. Any PR may contain untrusted code, and allowing an automation to run against that code and push to a branch might be a bit of a challenge to do safely, especially if we do not want to use privileged tokens. The fun part of running an OSS project I suppose. Let me know your thoughts on this; I don't yet know if I'll find time to explore that alternative in the near future. The reason I'd opted for this approach of suggestions is that it opens up further automated recipe runs, and helps nudge folks to better patterns without overwriting their changes just yet. |
These two workflows together ensure that if a pull request comes in that has formatting issues, that a
github-actionsbot then posts a code suggestion comment for fix any offending lines. Those comments would look something like thisI've used this to good effect on the OpenRewrite GitHub org, where we run some additional automations for copyright headers and coding best practices.
This has been discussed over the past year or so with @rwinch (👋🏻 ), as a good first step towards gently enforcing standards, as opposed to an unfriendly failing build pipeline with delayed feedback.
The PR leverages https://github.com/googleapis/code-suggester to turn a git diff into code suggestion comments, which we've found to work well for smaller formatting issues. https://github.com/reviewdog/reviewdog could be a possible alternative, as well as others.
I've of course not been able to test these new workflows, as it requires them to be available on the main branch first, but I've done my best to match up with the existing .github/workflows/pr-build-workflow.yml in terms of actions used. I figured to offer a first step PR here, but of course this could be generalized if desired.
No worries if you'd rather go a different direction. I figured a direct PR would be easier to review than yet another issues to discuss this once more.